feat: out-of-process host for existing Python CPEX plugins - #149
feat: out-of-process host for existing Python CPEX plugins#149tedhabeck wants to merge 5 commits into
Conversation
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
Signed-off-by: habeck <habeck@us.ibm.com>
araujof
left a comment
There was a problem hiding this comment.
Nice work!
Potential blockers:
The extensions merge writes a plugin's filtered view back over canonical state. extensions.rs:265 starts from inbound.cow_copy(), which is the capability-filtered view, and the custom slot is accepted unconditionally and sets applied on its own. merge_owned then swaps whole slots (cpex-core container.rs:238). Meanwhile the executor's only content check is the label superset, gated on read_labels, with a bare else that merges anyway (executor.rs:463). So a plugin with no security capabilities can wipe the pipeline's security labels just by returning a custom value, and with a labels token it can rewrite subject and auth_method too. Delegation isn't validated at all. The description says the executor enforces the mutability tiers; it does, but per slot rather than per field, and that's the gap. Merging field by field from the inbound value on the three gated slots should close it.
Nothing checks that the worker understands what we're sending. plugin.rs:433 spawns and returns. An older worker.py silently ignores the credential field, so a plugin that asked for credentials runs with none, which is what fail-closed rule 2 exists to prevent. Given the 0.1.2 version collision you documented, that's reachable in production, not just in tests. The grep in testing.rs would do for now; a capabilities task type would be better.
Related, and probably why both of those survived this long: the test suite reports ok while skipping. All four credential_e2e tests skip and pass, including the negative ones, and 10 of 20 integration tests do the same. Nothing runs --ignored in CI. I only caught it by running with --nocapture. I'd fix that first, since right now it reports safety that isn't there.
Other findings:
Set-Cookieisn't inSENSITIVE_HEADERSalthough the comment says it is (extensions.rs:91), and both strip tests assert withis_sensitive, so they can't fail.- The returned http slot blanks method, path, host and scheme (
:297). Policies gate on those per CHANGELOG 0.2.2, so it's a bit worse than the correctness bug you flagged. max_content_sizeis only checked outbound (worker.rs:251).reader_loopandstderr_loopare unbounded.- The delegated token pick filters on mode but not audience (
credentials.rs:324), so map order decides which token a plugin gets. - A dead worker is never replaced (
plugin.rs:328), and there's no timeout on pip install (venv.rs:445), which stalls startup since init is sequential. - Two plugins sharing a package will delete each other's venv (
venv.rs:423). extensions-wire-contract.mddoesn't exist anywhere, on disk or in any commit. It's cited throughout the description as the reference, so I had nothing to check the contract against. The real material is incmf-message-spec.mdsection 3.
The raw-credentials reversal I'd rather have a decision on than a fix. cpex-core raw_credentials.rs:37 says handlers needing raw material must run in-process, and this deliberately undoes that. Your write-up is honest about the residual exposure, so it's really whether we accept it. Either way the two crates shouldn't be left asserting opposite invariants.
Smaller: zeroize is declared and never used, error.rs:126 throws away the pip exit code and stderr into an empty details map, the crate isn't actually in default-members despite its comment, CHANGELOG wasn't updated, and gitignore's plugins/ is unanchored so it swallows builtins/plugins/.
Summary
Closes: #20
Adds
cpex-hosts-python, the crate that lets the RustPluginManagerrun thePython plugins that already exist — PII filters, identity resolvers, token
delegators — without rewriting them. It builds a per-plugin virtualenv, launches
the framework's
worker.pyin it as a long-lived subprocess, and speaks the samenewline-delimited-JSON stdio protocol the Python CLI already uses.
The point is migration continuity: a gateway can move to the Rust manager while
its existing Python plugins keep working unchanged.
Why
The Rust
PluginManagercannot run a Python plugin on its own. This crate is theRust counterpart to the Python CLI's
client.py+venv_comm.py. Rather thanport those as a one-to-one translation, it splits the three concerns — venv
build/cache, worker subprocess/protocol, and per-hook serialization — so each
tests in isolation and shared-package venv handling falls naturally to the venv
manager.
Venv construction and worker launch happen in
initialize()(rollback onfailure), teardown in
shutdown(). Neither belongs on the invoke path — a coldpip install is measured in minutes.
What's here
Venv lifecycle (
venv.rs) — build, cache, and resolve per-plugin venvs,including shared-package layouts. Cache metadata is keyed per host class.
Worker protocol (
worker.rs) — subprocess supervision and the NDJSON stdioframing, with the
max_content_sizeframe bound enforced on both sides.Hook dispatch (
plugin.rs,conversion.rs,legacy/) — one adapter perdeclared hook, carrying the native Pydantic payload shapes (
ToolPreInvokePayloadand friends) rather than a generic wrapper, so existing plugins validate as-is.
Credentials (
credentials.rs) — the framework strips raw tokens at everyprocess boundary (token fields are
#[serde(skip)]), but identity and delegationplugins genuinely need them. This adds a capability-gated wire DTO: a plugin
declaring
read_inbound_credentialsorread_delegated_tokensgets a dedicatedcredentialobject built by reading the in-memory token directly. Productioncredential types keep their serde guard and are never serialized. Fail-closed
rules and the residual exposure this does not close are documented in the
module.
Extensions delivery (
extensions.rs) — the executor's capability-filteredExtensionsis serialized onto the task, so a 3-arg(payload, context, extensions)hook sees out-of-process what it would see in-process. Returns comeback through the executor's existing copy-on-write merge, which enforces the
mutability tiers; this host adds no tier logic of its own. Sensitive headers
(
Authorization,Cookie,X-API-Key) are stripped in both directions,case-insensitively, and
raw_credentialsnever rides this channel.Tests
172 passing (152 unit + 20 integration), 2
#[ignore]d by default because theyneed an installed plugin with a built venv.
isolated_venv_e2e.rscredential_e2e.rsextensions_merge_e2e.rsconfig_e2e.rsinvoke_by_name→ executor → worker, against a plugin installed the way an operator installs oneextensions_merge_e2e.rsis worth a note on test design. The accept-side tieroutcomes can't be unit tested:
http,security, anddelegationwrites aregated by a
WriteTokenwhose constructor ispub(crate)tocpex-core, so thiscrate cannot mint one even in a test — which is exactly the property that makes
the gate trustworthy. Those paths are therefore driven through a real pipeline,
with tokens minted by the executor from declared capabilities. A
fake_workerstand-in covers the same merges without a Python subprocess by calling the
production parser on a canned response, so the code under test is identical and
only the subprocess is replaced.
Cross-surface verification, and one finding
The host and the Python worker land on different branches and ship separately, so
docs/specs/extensions-wire-contract.mdpins the contract they share — neitherside's source can serve as the other's reference.
This branch ran the two against each other for the first time, via populated
Extensionson thetool_pre_invokehook inconfig_e2e.rs. The channel works:extensions cross into a real worker subprocess, are reconstructed there, and the
hook runs. It also overturned two claims the spec had recorded about the known
http-slot divergence, in the reassuring direction.The divergence itself is unchanged and known — Rust
HttpExtensionhasrequest_headers/response_headers, Python has a singleheaders. The spec saida Rust-shaped slot fails validation in the Python model and is dropped with a
warning. Both claims are wrong. Neither model sets
extra="forbid"/deny_unknown_fields, and both default their header maps, so unknown keys aresilently ignored and missing ones default to empty:
{"request_headers": {"X-Request-Id": "r1"}}HttpExtension(headers={}){"headers": {"X-Fine": "yes"}}request_headers: {}, response_headers: {}Verified empirically against both models, not inferred. The slot arrives
present-but-empty — the "hollow slot" this same contract explicitly rejects for
raw_credentials. A plugin readsextensions.http.headers, gets{}, and cannotdistinguish "no headers on this request" from "the headers didn't cross."
Two follow-on corrections went into the spec:
reconstruct_extensionscallsmodel_validateon the whole object, so agenuinely-failing slot would zero out all extensions, not degrade per-slot as
the doc implied.
PipelineResult::modified_extensionsis the pipeline's final view and isalways
Some(...)on an allowed pipeline — not a "something changed" flag.Its own doc comment in
executor.rssays otherwise; noted in the spec, commentleft alone as out of scope here.
This is a correctness bug, not a security one. The sensitive-header strip runs
pre-serialization against the fields the sending model actually declares, so
Authorizationnever reaches the wire regardless of which shape the receiverexpects.
config_e2e.rsasserts exactly that: no credential header value appearsanywhere in the serialized task JSON.
Known gaps
Tracked as Remaining work in the spec:
httpslot needs a mapping on one side. The other eleven slots crosscorrectly.
that let the divergence stay misdescribed: existing tests assert either on the
wire JSON (Rust-shaped, so it passes) or on the hook running (it does). Write
it as a failing test, then fix (1).
mismatch into a silent one. Tightening trades the deliberate slot-level
version-skew tolerance, so it's a real design call rather than an obvious win.
important caveat on the verification above.
On (4), specifically
The worker side is unreleased (#113). Reproducing the run needs the plugin installed
and its venv's
cpexreplaced by hand:The version numbers collide. The branch build self-reports
0.1.2, and PyPIpublishes a
cpex0.1.2 that is a different artifact with no extensions support(no
EXTENSIONS_FIELD, noreconstruct_extensions). Nothing inpip listor thedist-info version distinguishes them, so a venv can look correctly provisioned and
silently have no extensions channel. Check
direct_url.json, not the version.The consequence for reviewers:
config_e2e.rscurrently guards only on the venvexisting, so a registry-provisioned venv yields a green test that proves
nothing — the worker ignores the unknown
extensionsfield, the hook still runs,the marker still lands. The guard should grep the installed
worker.pyforEXTENSIONS_FIELD, astesting::worker_delivers_extensionsalready does for theCPEX_PYTHON_SOURCEpath. Until then, treat a greenconfig_e2e.rsasconditional on having checked the venv's provenance.
Before merging
The working tree carries untracked artifacts that should be sorted out — none are
gitignored:
docs/specs/extensions-wire-contract.mdand thedocs/plans/+docs/brainstorms/files — these are worth committing; the spec isreferenced throughout this description.
Checks
make lintpassesmake testpasses